-
-
Notifications
You must be signed in to change notification settings - Fork 747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use regex-lint to provide circular dependency checking in pants #5778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
- name: must_not_import_st2common | ||
pattern: st2common | ||
inverted: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does inverted mean here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Search for st2common
but mark files as successful if they do NOT have the string.
7b1b0d6
to
4915cc2
Compare
4915cc2
to
5b02ca5
Compare
4bcf7fa
to
5958ffd
Compare
5958ffd
to
eeb48b9
Compare
eeb48b9
to
de4042f
Compare
Background
This is another part of introducing
pants
, as discussed in the TSC Meetings on 12 July 2022, 02 Aug 2022, 06 Sept 2022, and 04 Oct 2022. Pants has fine-grained per-file caching of results for lint, fmt (like black), test, etc. It also has lockfiles that work well for monorepos that have multiple python packages. With these lockfiles CI should not break when any of our dependencies or our transitive dependencies release new versions, because CI will continue to use the locked version until we explicitly relock with updates.To keep PRs as manageable/reviewable as possible, introducing pants will take a series of PRs. I do not know yet how many PRs; I will break this up into logical steps with these goals:
pants
to the st2 repo, andpants
step-by-step.Other pants PRs include:
pants_ignore
and bump pants to v2.14.0rc1 #5733BUILD
files with./pants tailor ::
#5738Overview of this PR
The Makefile has some checks for circular dependencies. Pants has a builtin
regex-lint
subsystem that lets us do basically the same thing via pants.These are the relevant targets in the Makefile:
st2/Makefile
Lines 578 to 591 in 94d0798
It turns out that the Makefile targets are somewhat out-of-date: For example, some of the listed exceptions to the rules no longer apply and we removed st2exporter in #5676. I documented each of my updates in comments of the lint config file.
In #5776, I had to add our
st2flake8
plugin forflake8
which handles checking for the copyright. We can replace that plugin withregex-lint
. I noted how we can do that in the regex-lint config file, but I left it commented out for now. We can enable it later when we're ready to retire thest2flake8
custom plugin, which we can probably do once we're ready to remove the Makefile and related files.Relevant Pants documentation
./pants generate-lockfiles
goal./pants lint
goalThings you can do with pantsbuild
You can run
./pants lint ::
to see if regex-lint finds any issues (the GHA Lint workflow runs this as well). You can run only one of the linters with the helpful--only
arg like this:The
--only
flag is "scoped" to the lint goal. So, these are equivalent if you wanted to specify that flag in a different order:./pants lint --only=regex-lint ::
./pants --lint-only=regex-lint lint ::
./pants lint :: --lint-only=regex-lint
note: that I had to add
lint
in the arg name to use a different order